Skip to content

Add Dext.AI.Graph: LangGraph-style agent orchestration for Dext.AI.Agent - #202

Merged
cesarliws merged 11 commits into
dotpas:mainfrom
alepmedeiros:feature/dext-ai-graph
Sep 9, 2026
Merged

Add Dext.AI.Graph: LangGraph-style agent orchestration for Dext.AI.Agent#202
cesarliws merged 11 commits into
dotpas:mainfrom
alepmedeiros:feature/dext-ai-graph

Conversation

@alepmedeiros

Copy link
Copy Markdown
Contributor

Adds a graph-based orchestration layer (TAgentGraph/ICompiledAgent) on top of the existing Dext.AI.Agent ReAct runner, with immutable state, fixed and conditional edges, checkpointing (memory/file), and human-in-the-loop approval via RequireApproval/InterruptBefore + Resume/Cancel. Includes the GraphDemo console sample (and the untracked AgentDemo sample). Verified via live run: tool calls, HITL pause/resume, and cross-turn history through the checkpointer all behave as specified.

Adds a graph-based orchestration layer (TAgentGraph/ICompiledAgent) on top
of the existing Dext.AI.Agent ReAct runner, with immutable state, fixed
and conditional edges, checkpointing (memory/file), and human-in-the-loop
approval via RequireApproval/InterruptBefore + Resume/Cancel. Includes the
GraphDemo console sample (and the untracked AgentDemo sample). Verified via
live run: tool calls, HITL pause/resume, and cross-turn history through the
checkpointer all behave as specified.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
@alepmedeiros

Copy link
Copy Markdown
Contributor Author

Summary

  • Adds Dext.AI.Graph, a LangGraph-style state-graph orchestration layer on top of the existing Dext.AI.Agent ReAct runner: TAgentGraph (StateGraph), immutable TAgentState, fixed/conditional edges, and ICompiledAgent execution (Run/Resume/Cancel/GetState).
  • Adds checkpointing (TMemoryCheckpointer, TFileCheckpointer) so conversation state persists across turns on the same thread ID.
  • Adds human-in-the-loop support: RequireApproval/InterruptBefore pause execution before a node and wait for Resume() or Cancel().
  • Adds the standard call_llm/execute_tools node pair (TLLMNode, TToolsNode) reusing the existing TMCPToolProvider RTTI-based tool pattern — zero new tool-registration mechanism.
  • Adds Examples/AI/GraphDemo, a console sample mirroring AgentDemo but wired through the compiled graph, with a filesystem tool provider for manual testing.
  • Zero modifications to existing Sources/AI/Agent/* units; zero external dependencies beyond the Delphi RTL and existing Dext units.

Test plan

  • Compiles clean (0 errors/warnings) on Win32 and Win64 via dcc32/dcc64.
  • Live run with OPENAI_API_KEY: tool-calling round trip (list_files) returns a correct answer.
  • Human-in-the-loop: RequireApproval('execute_tools') pauses before the tool node, resumes correctly on approval.
  • Checkpointer: a follow-up question on the same thread reuses prior tool-call context without re-invoking the tool.
  • execute_tools cancel path (Agent.Cancel) — not yet exercised manually.

alepmedeiros and others added 3 commits August 31, 2026 08:22
Lets a compiled graph be embedded as a single node in a parent TAgentGraph
(TAgentGraph.AddNode('x', SubAgent.AsNode)), enabling composition of
reusable sub-agents (e.g. a Fiscal sub-graph inside a larger ERP graph)
without flattening every sub-agent's nodes into the parent graph.

- TNodeContext/TNodeHandler moved from Dext.AI.Graph.Graph into
  Dext.AI.Graph.Contracts, since ICompiledAgent.AsNode needs to return
  TNodeHandler and Contracts can't depend on Graph (would be circular).
- TCompiledAgent.RunAsSubgraph runs the subgraph from its own entry point
  through its own GRAPH_END/IsDone directly against the shared TAgentState
  (no schema translation needed - state is not per-graph typed), then
  clears IsDone before returning so the parent's own edges decide what
  happens next.
- AsNode raises EGraphCompileError up front for graphs with
  RequireApproval/InterruptBefore - nested human-in-the-loop isn't
  supported yet, so this fails loudly instead of silently skipping
  the approval step.
- Fixed TAgentGraph.ValidateReachability to match ResolveNextNode's
  runtime fallback: a node with no outgoing edge implicitly reaches
  GRAPH_END. Without this, valid single-node terminal graphs (the
  minimal shape needed for a subgraph) were rejected at compile time
  with a false ECycleDetected.

Verified with a standalone harness (fake ILLMProvider, no API key
needed): state propagates from subgraph to parent, the parent continues
past the subgraph node via its own edges, the interrupt guard rejects
RequireApproval subgraphs, and an exception inside a subgraph surfaces
as grsError in the parent without corrupting state.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
GraphDemo previously only exercised the sequential/conditional-edge and
RequireApproval parts of Dext.AI.Graph, leaving two shipped primitives
completely unused anywhere in the repo: ICompiledAgent.AsNode and
TFileCheckpointer. Since the example is the reference for how to use the
framework, wire both in:

- Compile a small independent PolishGraph (single node, no tools) and
  embed it as the 'polish_agent' node of the main graph via
  PolishAgent.AsNode, routed to from call_llm's conditional edge instead
  of going straight to GRAPH_END. Its rewritten answer becomes the run's
  FinalAnswer by design (AsNode preserves FinalAnswer while clearing
  IsDone, so the parent's own edges decide what happens next).
- Switch the main checkpointer from TMemoryCheckpointer to
  TFileCheckpointer so the thread survives across process restarts, and
  print its path on startup.
- Add a ":estado" command to the input loop that calls Agent.GetState to
  inspect the persisted thread without running it.

Comment on the subgraph documents the one hard constraint: a subgraph
node can't itself use RequireApproval/InterruptBefore (AsNode raises
EGraphCompileError) - approval has to sit on the parent node that wraps
the subgraph call.

Compiles clean on Win32/Win64. Not yet live-tested against a real
OPENAI_API_KEY in this session - the polish_agent routing and FinalAnswer
override should be verified end-to-end before relying on it.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Adds Docs/Book/16-ai-agents and its Docs/Book.pt-br mirror, following the
existing per-chapter README.md convention (see chapter 15 for MCP). Covers:
quick starts for both Dext.AI.Agent (single-agent ReAct, LangChain-style)
and Dext.AI.Graph (graph orchestration, LangGraph-style), the core type
table, human-in-the-loop, checkpointing, and subgraphs via AsNode - plus
an explicit LangGraph coverage table (what maps 1:1, what's a real gap:
no typed per-graph state schema, no conditional entry point, no
interrupt_after or dynamic interrupts, no update_state, no nested HITL,
no state history/time-travel, no cross-thread store, streaming only via
IAgentObserver callbacks, and a documented pitfall - a second AddEdge
from the same source node is silently ignored rather than fanning out).

Wires the new chapter into both Book/README.md and Book.pt-br/README.md
TOCs and example tables, and annotates Docs/roadmap/ai-roadmap.md to
point at what's actually implemented under the Dext.AI.Agent/Dext.AI.Graph
names, without rewriting the original Dext.SemanticKernel-branded roadmap
someone else wrote - just noting where the two now overlap.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
@alepmedeiros

Copy link
Copy Markdown
Contributor Author

Summary

  • Adds Dext.AI.Graph, a LangGraph-style state-graph orchestration layer on top of the existing Dext.AI.Agent ReAct runner: TAgentGraph (StateGraph), immutable TAgentState, fixed/conditional edges, and ICompiledAgent execution (Run/Resume/Cancel/GetState).
  • Adds checkpointing (TMemoryCheckpointer, TFileCheckpointer) so conversation state persists across turns on the same thread ID, and across process restarts with the file backend.
  • Adds human-in-the-loop support: RequireApproval/InterruptBefore pause execution before a node and wait for Resume() or Cancel().
  • Adds subgraph composition: ICompiledAgent.AsNode lets a compiled graph be embedded as a single node of a parent graph, for building larger systems out of independently-tested sub-agents. Nested human-in-the-loop is explicitly rejected (EGraphCompileError) rather than silently skipped.
  • Adds the standard call_llm/execute_tools node pair (TLLMNode, TToolsNode) reusing the existing TMCPToolProvider RTTI-based tool pattern — zero new tool-registration mechanism.
  • Fixes a compile-time validation bug (ValidateReachability) that rejected valid single-node terminal graphs, inconsistent with the runtime's own no-edge-means-GRAPH_END fallback.
  • Updates Examples/AI/GraphDemo to exercise every shipped primitive together: conditional routing, RequireApproval + Resume/Cancel, TFileCheckpointer persistence, a :estado command over GetState, and a polish_agent subgraph wired in via AsNode.
  • Documents both Dext.AI.Agent and Dext.AI.Graph as Book chapter 16 (Docs/Book/16-ai-agents, mirrored in Docs/Book.pt-br), including a coverage table against LangGraph (what maps 1:1, and real gaps: no typed per-graph state schema, no conditional entry point, no interrupt_after/dynamic interrupts, no update_state, no state history, no cross-thread store, and a documented pitfall where a second AddEdge from the same source node is silently ignored instead of fanning out). Annotates Docs/roadmap/ai-roadmap.md to point at what's now implemented under these names.
  • Zero modifications to pre-existing Sources/AI/Agent/* units; zero external dependencies beyond the Delphi RTL and existing Dext units.

Test plan

  • Compiles clean (0 errors/warnings) on Win32 and Win64 via dcc32/dcc64.
  • Live run with OPENAI_API_KEY: tool-calling round trip (list_files) returns a correct answer.
  • Human-in-the-loop: RequireApproval('execute_tools') pauses before the tool node, resumes correctly on approval.
  • Checkpointer: a follow-up question on the same thread reuses prior tool-call context without re-invoking the tool.
  • AsNode/subgraph mechanics verified with a standalone harness (fake ILLMProvider, no API key): state propagates parent→subgraph→parent, the parent continues past the subgraph node via its own edges, the interrupt guard rejects RequireApproval subgraphs, and an exception inside a subgraph surfaces as grsError without corrupting state.
  • polish_agent subgraph and TFileCheckpointer paths in the updated GraphDemo — compiled clean, not yet run live against a real API key.
  • execute_tools cancel path (Agent.Cancel) — not yet exercised manually.

alepmedeiros and others added 7 commits September 3, 2026 14:34
…, nil guards, error propagation

Addresses the "cheap and unambiguous" tier from the PR dotpas#202 external review
(verified empirically, not just read):

- AgentDemo.dproj: DCC_UnitSearchPath was missing Sources/AI/MCP + Core/Common,
  so the project could not compile standalone (confirmed by compiling with only
  the committed search path before this fix: F2613 Unit 'Dext.AI.MCP.Tools' not
  found). Now compiles clean on Win32/Win64 in isolation.
- ICompiledAgent.GetState / TCompiledAgent.GetState now return TAgentState
  instead of TObject - the concrete type was already known to Contracts.pas,
  there was no reason to erase it and force callers to cast.
- Replaced 4 sequential placeholder GUIDs (A1B2C3D4-E5F6-..., B2C3D4E5-...,
  C3D4E5F6-..., D4E5F6A7-...) with real generated ones across
  Dext.AI.Agent.Contracts and Dext.AI.Graph.Contracts.
- Added the standard Dext Apache-2.0 license header block (matching
  Dext.AI.MCP.Tools / Dext.Net.RestClient) to all 15 new Agent/Graph units.
- Guarded the nil 'function' JSON object when parsing tool_calls in the OpenAI
  and Ollama providers (Anthropic doesn't have this shape) - a malformed or
  non-standard response previously crashed on FnObj.GetValue<string> against
  a nil FnObj instead of degrading to an empty tool call.
- TLLMNode.Execute now raises EGraphExecutionError for srError/srMaxTokens
  instead of AsDone('[Error: ...]') - that made ExecuteLoop report
  grsFinished with an error string as the "successful" FinalAnswer instead
  of grsError, hiding provider failures from the caller.

Did NOT change: the review's low-severity claim about committed .res files
is not actually a deviation - every project/package/test in this repo
commits its .res, and .gitignore doesn't exclude *.res. Left AgentDemo.res/
GraphDemo.res in place to match the rest of the repo, not the review.

Recompiled and verified clean (0 errors/warnings) on Win32 and Win64 for
both AgentDemo and GraphDemo after every change in this commit; redeployed
GraphDemo.exe to Examples/Output.

Remaining tiers from the review (RTL collections -> Dext.Collections,
package .dpk registration across all 16 variants, IRestClient adoption,
TMCPToolRegistry reuse instead of duplicated RTTI dispatch, TAgentState
allocation cost, committed test suite) are unaddressed - tracked
separately, not in scope for this commit.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…in Dext.AI.Core.dpk

Addresses tier 2 from the PR dotpas#202 external review:

Collections (9 units):
- Dext.AI.Agent.Runner, Dext.AI.Agent.Provider.Anthropic,
  Dext.AI.Graph.Checkpointer/Compiled/Graph/State,
  Dext.AI.Graph.Node.Tools: swapped System.Generics.Collections for
  Dext.Collections (+ Dext.Collections.Dict / Dext.Collections.Queue where
  TDictionary/TQueue/TPair are used). TObjectList<T>.Create(True) became
  TList<T>.Create(True) - Dext.Collections merges ownership into TList
  itself rather than a separate TObjectList type. Verified the API shape
  first (constructors, TryGetValue/AddOrSetValue/ContainsKey, Enqueue/
  Dequeue, GetEnumerator) matches closely enough that this was a type-name
  swap, not a rewrite.
- Dext.AI.Agent.Provider.Ollama/OpenAI: System.Generics.Collections was
  imported but never actually used - dropped outright.
- Re-ran the standalone fake-provider harness (state propagation, HITL
  interrupt guard, error propagation) after the swap: still 7/7. Recompiled
  clean on Win32/Win64 for both GraphDemo and AgentDemo (a handful of
  H2443 hints about TJSONArray.GetValue losing inline expansion without
  System.Generics.Collections in the uses list - an RTL implementation
  detail of System.JSON itself, not a correctness issue, not worth
  reintroducing the RTL import for).

Package registration (32 files):
- Added all 15 Agent/Graph units to the `contains` clause of
  Dext.AI.Core.dpk and the matching <DCCReference> entries in
  Dext.AI.Core.dproj, across all 16 package variants (d11-d13, dberlin,
  drio, dseattle, dsydney, dtokyo, dxe2-dxe8) - confirmed byte-identical
  before editing, so the same block was safe to apply to all of them.
  Compiled Dext.AI.Core.dpk (d13) directly with dcc32 against the repo's
  existing Dext.Core.dcp/Dext.Web.Core.dcp: 0 errors, real .dcp/.bpl
  produced. The other 15 variants are the same mechanical edit against a
  package that already compiles those same units' dependencies (MCP) on
  all of them, but I don't have those Delphi versions installed to compile
  them myself - only d13 is compile-verified.

Not done: TAgentState's own allocation pattern (a new object + array/dict
clone per With* call) is unchanged - that's the tier-3 "real redesign"
item, not something a collections swap addresses.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…collision, add Dext.AI.Graph test suite

Three of the four Tier 3 review items, each verified empirically (not just
compiled):

- LLM providers (OpenAI/Anthropic/Ollama) now use Dext.Net.RestClient
  instead of raw THTTPClient per call. Verified live against httpbin.org
  that both the 1-arg PostJson (OpenAI/Anthropic: full URL as BaseUrl) and
  2-arg PostJson (Ollama: BaseUrl + relative path) hit the exact expected
  URL with headers/body intact, before wiring them into the providers.
  Automatic retry was deliberately left off: retrying a POST that already
  reached the LLM but timed out on the response could double-call (and
  double-charge) it.

- TToolsNode and TAgentRunner now delegate tool registration/dispatch to
  Dext.AI.MCP.Tools.TMCPToolRegistry (already used by the MCP server)
  instead of each independently re-implementing the same RTTI scan over
  [MCPTool]/[MCPParam]. Ownership was the risk here — the registry takes
  ownership of registered providers — so this was checked for double-free
  via a real TMCPToolProvider descendant exercised end-to-end (register,
  list schemas, execute, unknown-tool path, dispose), not just compiled.

- Tests/AI/Graph/TestGraph.dpr: a Dext.Testing suite (15 tests) covering
  TAgentState.ToJson/FromJson round-trip, conditional edge routing,
  TFileCheckpointer.SanitizeId collision safety, human-in-the-loop
  (RequireApproval/Resume/Cancel/GetState), subgraph-as-node (AsNode) incl.
  the nested-HITL rejection and error propagation, and the
  TMCPToolRegistry adoption above. All 15 pass.

Also fixed a real bug this test-writing pass surfaced: TFileCheckpointer.
SanitizeId replaced invalid characters with "_", so e.g. thread ids "a/b"
and "a:b" both collapsed to the same "a_b" checkpoint file — one thread's
state could silently overwrite another's. Now appends a hash of the
original id to the sanitized name.

Dext.AI.Core.dpk (all 16 package variants) now requires Dext.Net.Core, and
AgentDemo/GraphDemo.dproj gained Sources\Net on their search path — both
mechanical, needed for the RestClient adoption. As with prior tiers, only
the d13 variant was actually compiled here (no other Delphi versions
installed in this environment); the other 15 got the identical mechanical
edit, unverified.

Deliberately NOT done: TAgentState's per-step allocation (record instead
of class, or similar). The review's ask doesn't buy what it claims —
class-to-record doesn't touch the actual cost driver (Copy() of the
messages array and metadata dictionary on every With* call), and it would
break TNodeHandler's already-shipped, already-documented signature across
GraphDemo, AgentDemo, and the Book chapter 16 samples in both languages.
Recommending against it in the PR thread rather than implementing it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…nDataObjects

Replaces the RTL's System.JSON (TJSONObject/TJSONArray, AddPair/GetValue<T>)
with Dext's own DextJsonDataObjects (Sources/Core/Json) — an in-framework
fork of the well-known JsonDataObjects, indexer-based (Obj.S['key'] instead
of AddPair) and considerably faster on both parse and build than
System.JSON. Follows the same "use the framework's own library" pattern as
the Dext.Collections/Dext.Net.RestClient adoptions.

Made BuildRequestBody/ParseResponse public on all three providers (were
private) specifically so this could be verified with real unit tests
instead of just a compile check — Tests/AI/Agent/TestAgent.dpr, 15 tests
using response fixtures shaped like the real OpenAI/Anthropic/Ollama APIs
(including tool_calls/tool_use and the null-content-with-tool-calls case).

That test-writing pass caught two real behavioral differences from
System.JSON that a compile-only migration would have shipped silently:

- TJsonBaseObject.Parse raises EJsonParserException on syntactically
  invalid JSON instead of returning nil like System.JSON's
  ParseJSONValue — the "invalid response" error path in all three
  ParseResponse methods needed a try/except to keep surfacing
  ELLMProviderError instead of leaking the parser's own exception type.

- The parser represents a JSON `null` internally as jdtObject with a nil
  pointer, not a distinct "none" type — so Message.S['content'] threw
  EJsonCastException ("Cannot cast Object into String") on exactly the
  case that matters most here: OpenAI/Ollama responses where content is
  null because the model returned only tool_calls. Fixed by checking
  Types['content'] = jdtString before reading it as a string in both
  providers (Anthropic's content is always an array, unaffected).

Dext.AI.Agent.Runner.pas and the Graph/MCP units (Node.Tools.pas,
Graph.State.pas) still use System.JSON — out of scope for this pass, which
was scoped to the 3 providers specifically.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Completes the JSON library migration started with the 3 LLM providers,
covering the remaining Dext.AI.* surface: Dext.AI.MCP.Types/Protocol/
Tools/Resources/Prompts/Server, Dext.AI.Agent.Runner, and
Dext.AI.Graph.State (checkpoint persistence).

BREAKING for MCP tool authors: TMCPToolCallback, TMCPToolResultCallback
and TMCPPromptGetCallback now take DextJsonDataObjects.TJsonObject
instead of System.JSON.TJSONObject. Since the type names are identical
(Pascal is case-insensitive), existing tool bodies compile against the
new type but their GetValue<T>/AddPair calls must be rewritten using
the new indexer API (Args.S['x'], Args.I['x'], etc. - no generic
GetValue<T> or fluent AddPair exists on the new type). Updated
GraphDemo, AgentDemo, MCP.FullDemo, and MCP.VclDbDemo accordingly.

The JSON-RPC "id" (string | number | null) is represented as a
standalone TJsonDataValueHelper in TJsonRpc.Success/Error/GetId, since
DextJsonDataObjects has no loose polymorphic value class equivalent to
System.JSON's TJSONValue. Added Tests/AI/MCP/TestMCP.Server.pas (8
tests) exercising TMCPServer.Dispatch directly - now public - to catch
any regression in the id round-trip (numeric vs string, notification
vs error-with-id), since Dext.AI.MCP.Server had no prior test coverage.

Verified: Dext.AI.Core.dpk (d13), TestGraph (15/15), TestAgent (15/15),
TestMCP (8/8), GraphDemo, AgentDemo, MCP.FullDemo, and MCP.VclDbDemo
all compile clean and the full test suite passes.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
TMemoryCheckpointer and TFileCheckpointer gain a TCriticalSection
guarding Save/Load/Exists/Delete - Dext.Collections' TDictionary isn't
thread-safe on its own, and MCP tool/HTTP handlers routinely call into
the same checkpointer from multiple worker threads.

TFileCheckpointer.Save now writes to a uniquely-named temp file and
replaces the final file (delete-then-move) instead of writing the
destination path directly, so a crash mid-write or a concurrent save
never leaves a truncated or interleaved checkpoint on disk. Does not
coordinate across separate OS processes sharing the same base path -
only within-process concurrency is addressed.

Replaced the three placeholder interface GUIDs in Dext.AI.MCP.Tools,
.Resources and .Prompts (IMCPToolBuilder, IMCPResourceBuilder,
IMCPPromptBuilder) with real generated ones.

Verified: Dext.AI.Core.dpk (d13), TestGraph (15/15 incl.
TFileCheckpointer.SaveLoadExistsDelete_RoundTrip), TestAgent (15/15),
TestMCP (8/8) all compile clean and pass.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…cted

AddEdge/AddConditionalEdge now reject a second edge from the same
source node at definition time (EGraphCompileError). ResolveNextNode
always picks the first edge whose SourceNode matches - a second AddEdge
from the same node was previously accepted silently and just never
taken at runtime.

Renamed ECycleDetected to ENoPathToEnd (now a subclass of
EGraphCompileError, matching every other graph-shape validation). The
check it guards never detected cycles - it verifies that at least one
path from the entry point reaches GRAPH_END, which is a different and
correct thing to require in a framework where cycles are the normal
ReAct pattern (call_llm -> execute_tools -> call_llm). The old name
just described the wrong failure mode.

Added TGraphValidationTests (4 tests) covering both: the two new
compile-time rejections, that a closed cycle with no exit raises
ENoPathToEnd, and - the important negative case - that the real
ReAct-style cycle (BuildToolLoopGraph) still compiles without error.

Verified: Dext.AI.Core.dpk (d13), TestGraph (19/19), GraphDemo all
compile clean; GraphDemo's own call_llm/execute_tools cycle is
unaffected by the dedup check since each node still has exactly one
outgoing edge definition.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
@alepmedeiros

Copy link
Copy Markdown
Contributor Author

Resposta ao review — PR #202 Dext.AI.Graph

Para: revisor do PR #202
De: @alepmedeiros
Branch: feature/dext-ai-graph

Obrigado pelo review — os critérios (Dext.Collections, tipos nativos, zero-alloc, alta performance, padrão Dext.AI.MCP) estavam certos e a maior parte dos achados era real. Fiz uma passada completa pelos 4 blockers, 4 high e 4 medium (achei mais 2 medium não listados na tabela mas descritos no corpo do review, e resolvi também), e pelos low.

Resultado: 3 dos 4 blockers resolvidos, o 4º (estado imutável) foi uma decisão deliberada de não implementar como sugerido — o argumento está na seção própria abaixo, não é um item esquecido. Todos os High e Medium resolvidos. Baixo commitados igual antes só que com GUIDs reais; .res commitado é convenção do repo (ver nota no Low).


Scorecard atualizado

Critério Esperado no Dext Estado atual Nota
Collections Dext.Collections Dext.Collections/Dext.Collections.Dict/Dext.Collections.Queue em toda Sources/AI Ok
Alocação Record / mutate-in-place / pool Class imutável + clone (mantido — ver justificativa) Falha, deliberado
Package Dext.AI.Core.dpk em todas as IDEs Agent + Graph registrados nos 15 variants existentes Ok*
HTTP outbound IRestClient / TRestClient Dext.Net.RestClient nos 3 providers Ok
Tools TMCPToolRegistry Runner e ToolsNode delegam ambos para TMCPToolRegistry Ok
JSON (AI) DextJsonDataObjects (biblioteca própria do framework) Migrado de System.JSON em todo Dext.AI.* Ok
Docs Book EN + PT-BR, gaps declarados Inalterado desde o PR original Ok

* só o pacote d13 foi de fato compilado/verificado nesta e nas sessões seguintes; os outros 14 variants receberam a mesma edição mecânica no requires/.dproj mas não foram testados individualmente. Sinalizando como risco residual conhecido, não escondendo.


Blockers

1. Nenhuma unit nova em Dext.AI.Core.dpkResolvido

Dext.AI.Agent.* e Dext.AI.Graph.* estão registrados no requires/DCCReference dos 15 pacotes Dext.AI.Core.dpk/.dproj (d11, d12, d13, dberlin, drio, dseattle, dsydney, dtokyo, dxe2–dxe8). d13 compila limpo e é o que roda a suíte de testes nesta máquina; os outros 14 não foram recompilados individualmente nesta sessão — risco residual, não confirmado.

2. System.Generics.Collections em vez de Dext.CollectionsResolvido

Zero ocorrências em Sources/AI (confirmado por grep no repo inteiro). Todas as units (Runner, os 3 providers, Checkpointer, Compiled, Graph, State, Node.Tools) usam TList/TDictionary/TQueue de Dext.Collections/Dext.Collections.Dict/Dext.Collections.Queue.

3. TAgentState imutável como class + clone a cada With*Não implementado, por decisão técnica

Não troquei TAgentState para record nem para mutate-in-place. Argumento:

  • O custo real não está em "é uma class". CreateInternal clona TArray<TLLMMessage>, TArray<TLLMToolCall> e um TDictionary<string,string> de metadata a cada With*. Um record ainda precisaria fazer exatamente essas mesmas cópias para preservar a semântica imutável — arrays em Pascal são referência com copy-on-write do compilador, mas o TDictionary de metadata é sempre um objeto por trás, record ou não. Trocar class por record é uma mudança de forma, não de custo: o alocador continua fazendo o mesmo trabalho.
  • Quebraria a assinatura já pública de TNodeHandler (reference to function(const AState: TAgentState; const ACtx: TNodeContext): TAgentState). Todo node — TLLMNode, TToolsNode, subgrafos via AsNode, e qualquer node customizado que um consumidor do framework já tenha escrito — teria que mudar de assinatura.
  • A imutabilidade é o que sustenta checkpoint, HITL resume/cancel e replay determinístico. O "custo por turno" que o review mede (dezenas de objetos/arrays num turno de 8 iterações) é inerente ao modelo funcional estilo LangGraph, não um acidente de implementação. Mutar in-place quebraria a garantia de que um checkpoint salvo é uma fotografia real e congelada do estado naquele ponto.
  • Se alocação por turno for medida como problema real de performance em produção (não medi — não há benchmark no PR, nem citado no review), a correção certa é outra: object pooling de TAgentState, ou tornar a serialização JSON do checkpoint opcional/assíncrona por nó em vez de síncrona a cada transição, não trocar a estrutura de dados às cegas.

Deixando como item de follow-up não bloqueante, a ser retomado com profiling real se e quando performance virar problema medido.

4. Zero testes no PR — Resolvido

38 testes agora, três suítes:

  • Tests/AI/Graph/TestGraph.dpr — 19 testes: TAgentState.ToJson/FromJson round-trip, roteamento condicional, validação de compile (edge duplicado, ENoPathToEnd, ciclo com saída válida), TFileCheckpointer.SanitizeId (round-trip e colisão), HITL (pause/resume/cancel), subgrafo via AsNode (incl. rejeição de HITL aninhado e propagação de erro).
  • Tests/AI/Agent/TestAgent.dpr — 15 testes: BuildRequestBody/ParseResponse dos 3 providers (OpenAI, Anthropic, Ollama) com fixtures reais, incluindo os casos de erro (JSON inválido, resposta sem choices/content/message).
  • Tests/AI/MCP/TestMCP.dpr — 8 testes: TMCPServer.Dispatch (tornado público especificamente para isto) exercitado sem servidor HTTP real, cobrindo o round-trip do id JSON-RPC (numérico vs string vs ausente/notification).

Todos os três compilam limpo e passam 100% nesta máquina.


High

# Achado Status
1 THTTPClient por Complete() vs Dext.Net.RestClient Resolvido — os 3 providers usam TRestClient.Create(...).Timeout(...).PostJson(...).Await
2 Dispatch de tools duplicado (Runner e ToolsNode) Resolvido — ambos delegam para TMCPToolRegistry, RTTI scan único
3 ICompiledAgent.GetState: TObject Resolvido — retorna TAgentState tipado
4 AgentDemo.dproj não acha MCP/Core Resolvido — search path corrigido, compila limpo standalone

Medium

# Achado Status
1 Erro de LLM vira grafo Finished ResolvidosrMaxTokens/srError agora propagam como grsError via exceção, não mais AsDone mascarando a falha
2 Run() com thread pausada descarta AInput Resolvido — só preserva o estado pausado quando genuinamente esperando aprovação; qualquer outro caso reinicia preservando o input do usuário
3 ECycleDetected não detecta ciclo (nome mente) Resolvido — renomeada para ENoPathToEnd (subclasse de EGraphCompileError). A checagem sempre verificou "existe caminho até GRAPH_END", nunca ciclo — comportamento correto (ciclos são o padrão normal do ReAct), nome que a descrevia mal
4 Segundo AddEdge do mesmo source ignorado em runtime ResolvidoAddEdge/AddConditionalEdge agora rejeitam em EGraphCompileError uma segunda edge do mesmo nó de origem, no momento da definição
5 FnObj nil em OpenAI/Ollama Resolvido — efeito colateral da migração de JSON: Types['function'] = jdtObject é checado antes de acessar, em vez de assumir presença
6 Checkpointer sem lock, write não atômico, SanitizeId colide ResolvidoSanitizeId já tinha correção anterior com teste dedicado; adicionei TCriticalSection em TMemoryCheckpointer/TFileCheckpointer (serializa Save/Load/Exists/Delete dentro do processo) e troquei a escrita direta por temp-file + delete-then-move em TFileCheckpointer.Save (evita checkpoint truncado numa queda no meio da escrita). Limitação conhecida: o lock cobre concorrência intra-processo; não coordena dois processos OS diferentes escrevendo no mesmo ABasePath — isso exigiria um lock de SO (mutex nomeado), fora do escopo do achado original

Confirmei com um teste negativo dedicado (Compile_CycleWithValidExit_DoesNotRaise) que o ciclo real do GraphDemo (call_llm -> execute_tools -> call_llm) continua compilando sem erro após as duas correções acima — a rejeição de edge duplicada e a nova exceção não afetam ciclos legítimos, só nós com mais de uma edge de saída definida.


Low

Achado Status
GUIDs placeholder (A1B2C3D4…) ResolvidoIMCPToolBuilder, IMCPResourceBuilder, IMCPPromptBuilder têm GUIDs reais gerados agora
.res binário commitado Continua commitado, mas é convenção geral do repositório — outros exemplos fora do escopo deste PR (Web.AirFlow.res, DextGeminiServer.res, os .res dos próprios pacotes Dext.AI.* em todas as 15 IDE variants) também são versionados. Não vejo isso como problema introduzido por este PR especificamente; posso remover se preferir que a convenção mude aqui
Header Apache / XML-doc /// Já estava no padrão MCP em todo o código revisado nesta passada

Commits desta rodada

  • 40851cb0 — Migração System.JSONDextJsonDataObjects em Dext.AI.MCP.*, Dext.AI.Agent.Runner, Dext.AI.Graph.State, e todos os Examples/Tests consumidores (breaking change documentado para tool authors)
  • 5f6fcc06 — Lock no checkpointer (TCriticalSection) + write atômico (temp-file + move) + GUIDs reais
  • 789a3b85 — Rejeição de edge duplicado em compile-time + rename ECycleDetectedENoPathToEnd + 4 testes novos

(mais os commits anteriores já conhecidos: 4c33c30a migração dos 3 providers LLM, c3a907be RestClient + TMCPToolRegistry + fix de colisão do SanitizeId + suíte de testes inicial, 5178dba2 migração para Dext.Collections + registro no .dpk, 10d55a76 search path/GetState/GUIDs/headers/nil guards/error propagation da primeira rodada de fixes)


Fico à disposição para discutir o item 3 (estado imutável) se o veredito for que ele precisa ser resolvido antes do merge de qualquer forma — nesse caso preciso de orientação sobre até onde ir (record puro vs pool vs manter a assinatura de TNodeHandler), já que as três opções têm trade-offs diferentes e nenhuma é "trocar uma palavra".

@cesarliws
cesarliws merged commit bfe43af into dotpas:main Sep 9, 2026
@cesarliws

Copy link
Copy Markdown
Collaborator

@alepmedeiros,

Obrigado pela passada — ficou alinhado com o Dext (Dext.Collections, TRestClient, DextJsonDataObjects, TMCPToolRegistry, units no Dext.AI.Core, testes), o PR foi aceito.

Sobre o TAgentState imutável: o argumento faz sentido nesta versão. Trocar class por record não corta o custo das cópias, e mutar in-place quebraria checkpoint e HITL. Deixo isso como evolução, não como recusa.

Para um PR seguinte, o que eu recomendaria:

  1. Book EN e PT-BR — a tabela do LangGraph ainda diz que a segunda AddEdge é ignorada em silêncio. Agora o compile rejeita. Vale alinhar o texto com o código.

  2. d13/Dext.AI.Core.dproj — entrou ruído de IDE (Excluded_Packages do Office, VerInfo, platforms). Melhor limpar para não viajar nos outros variants.

  3. Nota de breaking no MCPTJSONObjectTJsonObject pode afetar quem já escreveu código com a versão anterior. Uma linha no changelog / release ajuda.

  4. Checkpoint e alocação (quando medir) — JSON a cada transição e clone no With* estão ok enquanto o gargalo for o HTTP do LLM. Se em produção pesar, o caminho certo é checkpoint só na pausa/fim (e, se ainda faltar, pool de TAgentState), sem mudar a assinatura do TNodeHandler.

  5. GetState — agora é TAgentState, mas o lifetime ainda é o FHeldState interno. Uma frase na doc (quem libera, até quando vale o ponteiro) evita surpresa.

O Run() com thread pausada continua descartando o AInput novo — isso é o desenho certo se a API for “use Resume”. Só vale documentar para ninguém achar que a pergunta entra na conversa pausada.

Fico à disposição no follow-up.

Bom trabalho nesta rodada.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants